spec: Fix inconsistency with transient publishes functionality (RTL11) #470
Conversation
mattheworiordan
changed the title
Given transient publishes, is "messages queued for send on that channel" still a useful concept? Why would a client library need a channel-specific queue now, rather than only have the connection-wide queue? (Especially if we want to have behavioural consistency with the proposed client#publish functionality, we need as little per-channel state as possible) Edit: answering my own question, it's still needed for presence actions per RTP16b |
| @@ -459,7 +459,7 @@ h3(#realtime-channel). Channel | |||
| ** @(RTL3b)@ If the connection state enters the @CLOSED@ state, then an @ATTACHING@ or @ATTACHED@ channel state will transition to @DETACHED@ | |||
| ** @(RTL3c)@ If the connection state enters the @SUSPENDED@ state, then an @ATTACHING@ or @ATTACHED@ channel state will transition to @SUSPENDED@ | |||
| ** @(RTL3d)@ If the connection state enters the @CONNECTED@ state, then a @SUSPENDED@ channel will initiate an attach operation and transition to @ATTACHING@. If the attach operation times out, the channel should return to the @SUSPENDED@ state (see "RTL4f":#RTL4f). | |||
| * @(RTL11)@ If a channel enters the @DETACHED@, @SUSPENDED@ or @FAILED@ state, then all messages that are still queued for send on that channel should be deleted from the queue triggering a failure for the publish or presence methods invoked for those messages | |||
| * @(RTL11)@ If a channel enters the @SUSPENDED@ or @FAILED@ state, then all messages that are still queued for send on that channel should be deleted from the queue triggering a failure for the publish or presence methods invoked for those messages | |||
There was a problem hiding this comment.
So: this spec item is now only useful for PresenceMessages, as Messages are no-longer per-channel queued. (Don't think we need it for connection-queued messages -- a channel isn't going enter the suspended or failed state when the connection isn't connected except as a result of the connection entering the suspended or failed states, at which point connection-queued messages will be failed anyway). But for PresenceMessages this change is wrong, they should be failed when the channel is detached.
How about:
* @(RTL11)@ If a channel enters the @DETACHED@, @SUSPENDED@ or @FAILED@ state, then all presence actions that are still queued for send on that channel per "RTP16b":#RTP16b should be deleted from the queue, and any callback passed to the corresponding presence method invocation should be called with an @ErrorInfo@ indicating the failure
There was a problem hiding this comment.
So: this spec item is now only useful for PresenceMessages, as Messages are no-longer per-channel queued.
Yes, I suppose that is correct.
Odd that we now have two mechanisms for delivering messages on channels, but it's necessary.
@paddybyers you happy with @SimonWoolf's suggested change?
There was a problem hiding this comment.
@paddybyers you happy with @SimonWoolf's suggested change?
lgtm
It doesn't. But unless I am mistaken, we don't detail that in our spec as that's an "internals" issue anyway?
Yup, OK. |
mattheworiordan
force-pushed
the
transient-publish-spec-fix-pr-323
branch
from
4be1ddf to
cf9286b
Compare
See https://github.com/ably/docs/pull/323/files Messages should no longer be NACK’ed when a channel becomes detached given we can continue to publish in the detached state. However, this does not apply to presence messages.
mattheworiordan
force-pushed
the
transient-publish-spec-fix-pr-323
branch
from
cf9286b to
a3d7171
Compare
See #323
Messages should no longer be NACK’ed when a channel becomes detached given we can continue to publish in the detached state